Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix migrate taxref command for false merge conflitcs #528

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft

Fix migrate taxref command for false merge conflitcs #528

wants to merge 2 commits into from

Conversation

jpm-cbna
Copy link
Contributor

@jpm-cbna jpm-cbna commented Aug 7, 2024 •
edited
Loading

See: #527

Two commites first 14532af with SQL refactoring, second 7304d09 with fixes.

@jpm-cbna jpm-cbna added the bug label Aug 7, 2024
@jpm-cbna jpm-cbna self-assigned this Aug 7, 2024
jpm-cbna
jpm-cbna commented Aug 8, 2024
View reviewed changes
...commands/migrate_taxref/data/changes_detection/1.2_taxref_changes_detections_cas_actions.sql
"action" = 'Conflicts with attributes : ' || atts
FROM conflict_atts_text AS a
WHERE a.f_cd_ref = c.f_cd_ref
AND cas = 'merge' ;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Est ce qu'il ne faudrait pas ajouter AND "action" IS NULL ici ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finalement, après relecture du code SQL, je dirai que non car le champ "action" est potentielle déjà rempli. Par exemple avec 'Update cd_ref no changes for attributes and medium'.

@jacquesfize
Copy link
Contributor

Salut @jpm-cbna,

La procédure de migration a subi des changements notoires dans la V2 de TaxHub, plus précisément à cause de la suppression de la table bib_noms #377.
Par conséquent, certains des problèmes que tu rencontres ne seront plus possible dans cette dernière version. Dans tous les cas, on analysera plus en détails ta proposition après la release de la V2.

@jpm-cbna
Copy link
Contributor Author

jpm-cbna commented Aug 9, 2024

La procédure de migration a subi des changements notoires dans la V2 de TaxHub, plus précisément à cause de la suppression de la table bib_noms #377. Par conséquent, certains des problèmes que tu rencontres ne seront plus possible dans cette dernière version. Dans tous les cas, on analysera plus en détails ta proposition après la release de la V2.

Oui, j'en ai bien conscience. C'est juste que j'avais besoin de comprendre d'où venaient les problèmes pour les corriger. Au final les corrections présentes dans le commit 7304d09 sont très simples et m'ont permis de réaliser proprement la migration.
Le commit de "refactorisation" 14532af est peut être inutile...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jacquesfize jacquesfize Awaiting requested review from jacquesfize

@amandine-sahl amandine-sahl Awaiting requested review from amandine-sahl

Assignees

@jpm-cbna jpm-cbna

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jpm-cbna @jacquesfize